Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zustand Proof of Concept #802

Draft
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Draft

Zustand Proof of Concept #802

wants to merge 4 commits into from

Conversation

sejas
Copy link
Member

@sejas sejas commented Jan 13, 2025

Note

This PR is not meant to be merged. It's just an exploration.

Related issues

Proposed Changes

  • Proof of concept that migrates ChatInput React Context to independent Zustand store

Testing Instructions

  • Test if the Assistant text input is saved independently for each site.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@sejas sejas marked this pull request as draft January 13, 2025 14:43
@sejas sejas self-assigned this Jan 13, 2025
@nightnei
Copy link
Contributor

nightnei commented Jan 14, 2025

I just recalled that I am using Zustand on https://ggwp.team/ 😄
Why? Because I asked ChatGPT "Recommend me some fancy state manager for React, I want to try it just for fun" 😁

The PoC looks cool, thanks for preparing it 👍

As I said on our weekly - I am a fan of MobX, it's classic, simple and stable solution. Also I tested it in action on big projects.
I am positive that Zustand can also do its job very well, so from my perspective, I am okay with any tool, as long as we achieve our main goal: having a state manager.

@fredrikekelund
Copy link
Contributor

fredrikekelund commented Jan 15, 2025

I played around with this some more and stand by my conclusion that Zustand isn't the best choice here.

MobX is more flexible in defining stores, which I think will benefit us by requiring less mental overhead if we adopt commonsensical conventions like stores being classes with properties and methods.

With Zustand, we can accomplish the same thing, but the store code is less readable because set calls are quite verbose, and deciding which functions should live in the store definition vs as auxiliary functions (using useChatStore.setState) is less intuitive.

I spent some time creating two equivalent stores based on ChatProvider in MobX and Zustand to compare them side by side:

A few initial observations:

  • Both options are an improvement over what we have today.
  • The resulting stores are relatively small (and relatively similar), so it's difficult to draw definitive conclusions. Still, I think my previous point about code organization in MobX holds up—it's more straightforward.
  • Type definitions are more tedious for the Zustand store. This might not always be apparent, but it'll generally be easier for TS to intuit types from a straightforward class, meaning less repetition when writing code.

Having thought about this for a few more days since our hangout on Monday, Redux is also still floating around my head as a potential contender—both because of its familiarity and because I don't think it'd be controversial in the same way that MobX or Zustand could potentially be.

@sejas
Copy link
Member Author

sejas commented Jan 15, 2025

Redux is also still floating around my head as a potential contender—both because of its familiarity and because I don't think it'd be controversial in the same way that MobX or Zustand could potentially be.

Yep! Redux is kind of the de facto library in mature software. Maybe a P2 could bring some light about others using a different library.

@fredrikekelund
Copy link
Contributor

This implementation could potentially be improved, but for comparison, here's a Redux version.

As pragmatic a choice as Redux may be, I can't help but recoil slightly when comparing them side by side like this because of the added complexity and overhead that Redux brings. I'm not saying it's the wrong choice, but still…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants